Skip to content

fix: avoid dependency comparison crashes#5999

Closed
alceops wants to merge 3 commits intoregro:mainfrom
alceops:alce/fix-5996-env-dep-comparison
Closed

fix: avoid dependency comparison crashes#5999
alceops wants to merge 3 commits intoregro:mainfrom
alceops:alce/fix-5996-env-dep-comparison

Conversation

@alceops
Copy link
Copy Markdown

@alceops alceops commented May 1, 2026

Summary

  • make _apply_env_dep_comparison() match dependency removals/updates by exact text first, then package token
  • skip missing dependency entries instead of raising ValueError
  • add focused regression coverage for bare CUDA package tokens and missing dependency text

Verification

  • python3 -m py_compile conda_forge_tick/update_deps.py tests/test_update_deps.py
  • git diff --check
  • focused pytest not run locally: this runner lacks repo test dependencies/pytest

Fixes #5996

@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.72%. Comparing base (e1e8c7d) to head (c88939c).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5999      +/-   ##
==========================================
+ Coverage   80.69%   80.72%   +0.02%     
==========================================
  Files         146      146              
  Lines       17443    17465      +22     
==========================================
+ Hits        14076    14098      +22     
  Misses       3367     3367              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@beckermr beckermr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change the style of the control-flow using the suggestions to keep the logic easier to parse.

Comment thread conda_forge_tick/update_deps.py
Comment thread conda_forge_tick/update_deps.py Outdated
if patch.after is None:
new_deps.pop(dep_index)
# Update existing package.
else:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else:

Comment thread conda_forge_tick/update_deps.py Outdated
# Update existing package.
else:
new_deps[new_deps.index(patch.before)] = patch.after
new_deps[dep_index] = patch.after
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
new_deps[dep_index] = patch.after
new_deps[dep_index] = patch.after

@leofang
Copy link
Copy Markdown
Contributor

leofang commented May 1, 2026

@alceops PTAL

@alceops
Copy link
Copy Markdown
Author

alceops commented May 1, 2026

Thanks, applied the requested control-flow cleanup in 266b061db14797c7c7891180e51cc9ba71d1f2cf: the removal path now continues and the update assignment is no longer nested under else.

Verification on the updated PR head:

  • python3 -m py_compile conda_forge_tick/update_deps.py tests/test_update_deps.py
  • git diff --check upstream/main...HEAD
  • hosted pre-commit.ci - pr status is success ✅

I did not claim a local pytest run; this runner is missing project test deps (networkx from tests/conftest.py).

@alceops alceops closed this May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_apply_env_dep_comparison crashes with ValueError when dep in cf_minus_df doesn't match recipe text exactly

3 participants